Skip to content

Use rand crate re-exports when available#64

Merged
stevenroose merged 1 commit intorust-bitcoin:masterfrom
michalkucharczyk:mku-use-reexports-fix
Mar 28, 2024
Merged

Use rand crate re-exports when available#64
stevenroose merged 1 commit intorust-bitcoin:masterfrom
michalkucharczyk:mku-use-reexports-fix

Conversation

@michalkucharczyk
Copy link
Copy Markdown
Contributor

Re-exports from rand crate shall be used. Otherwise trait bounds in Mnemonic::generate_in_with for rand::thread_rng object can get unsatisfied if crate deps get ouf of sync.

This commit is fixing following errors:

error[E0277]: the trait bound `ThreadRng: rand_core::RngCore` is not satisfied
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:292:30
    |
292 |         Mnemonic::generate_in_with(&mut rand::thread_rng(), language, word_count)
    |         -------------------------- ^^^^^^^^^^^^^^^^^^^^^^^ the trait `rand_core::RngCore` is not implemented for `ThreadRng`
    |         |
    |         required by a bound introduced by this call
    |::
    = help: the following other types implement trait `rand_core::RngCore`:
...
note: required by a bound in `Mnemonic::generate_in_with`
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:266:6
    |
260 |     pub fn generate_in_with<R>(
    |            ---------------- required by a bound in this associated function
...
266 |         R: rand_core::RngCore + rand_core::CryptoRng,
    |            ^^^^^^^^^^^^^^^^^^ required by this bound in `Mnemonic::generate_in_with`

error[E0277]: the trait bound `ThreadRng: rand_core::CryptoRng` is not satisfied
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:292:30
    |
292 |         Mnemonic::generate_in_with(&mut rand::thread_rng(), language, word_count)
    |         -------------------------- ^^^^^^^^^^^^^^^^^^^^^^^ the trait `rand_core::CryptoRng` is not implemented for `ThreadRng`
    |         |
    |         required by a bound introduced by this call
    |
    = help: the following other types implement trait `rand_core::CryptoRng`:
...
note: required by a bound in `Mnemonic::generate_in_with`
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:266:27
    |
260 |     pub fn generate_in_with<R>(
    |            ---------------- required by a bound in this associated function
...
266 |         R: rand_core::RngCore + rand_core::CryptoRng,
    |                                 ^^^^^^^^^^^^^^^^^^^^ required by this bound in `Mnemonic::generate_in_with`

@tcharding
Copy link
Copy Markdown
Member

Why is rand_core even used? rand v0.6 exposes RngCore and CryptoRng - why are we not just using them? @stevenroose do you remember?

@ggwpez
Copy link
Copy Markdown

ggwpez commented Feb 8, 2024

I also get this compile error. This would not happen if this crate had a Cargo.lock, or?

@tcharding
Copy link
Copy Markdown
Member

tcharding commented Feb 8, 2024

Can you share the build command that triggers this please?

@ggwpez
Copy link
Copy Markdown

ggwpez commented Feb 9, 2024

It happens when upstream dependencies pull in specific versions, but it can be reproduced here as follows.
Looking at the version requirements first, we have:

  • rand ">=0.4.0, <0.7.0"
  • rand_core ">=0.6.0, <0.9.0"

Now the following versions do fit these requirements:

  • rand 0.5.1
  • rand_core 0.8.5

But are incompatible with each other. Cargo does not complain about incompatible versions though, so when applying this change, it will fail to build:

 [dependencies]
-rand_core = { version = ">=0.4.0, <0.7.0", optional = true }
-crate_rand = { package = "rand", version = ">=0.6.0, <0.9.0", optional = true }
+rand_core = { version = "0.5.1", optional = true }
+crate_rand = { package = "rand", version = "0.8.5", optional = true }
 serde = { version = "1.0", default-features = false, features = [ "alloc" ], optional = true }

@michalkucharczyk is this how you encountered it as well?

@michalkucharczyk
Copy link
Copy Markdown
Contributor Author

I believe so, probably something similar. deps involved on my side were:

  • rand 0.5.1
  • rand 0.6.4
  • rand_core 0.8.5

@stevenroose
Copy link
Copy Markdown
Collaborator

Why is rand_core even used? rand v0.6 exposes RngCore and CryptoRng - why are we not just using them? @stevenroose do you remember?

I don't remember, I think in some old version of rand, there was no rand_core and now these traits live in rand_core? I think we should probably only need one of them, but not sure which, I'd say rand_core?

@stevenroose stevenroose mentioned this pull request Feb 9, 2024
@stevenroose
Copy link
Copy Markdown
Collaborator

Can you rebase on master to pass CI?

@tcharding
Copy link
Copy Markdown
Member

Ok, so to keep the loose dependency requirements we have to do this, that is probably better than tightening the dependency requirements.

Re-exports from `rand` crate shall be used. Otherwise trait bounds in
`Mnemonic::generate_in_with` for `rand::thread_rng` object can get
unsatisfied if crate deps get ouf of sync.

This commit is fixing following errors:
```
error[E0277]: the trait bound `ThreadRng: rand_core::RngCore` is not satisfied
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:292:30
    |
292 |         Mnemonic::generate_in_with(&mut rand::thread_rng(), language, word_count)
    |         -------------------------- ^^^^^^^^^^^^^^^^^^^^^^^ the trait `rand_core::RngCore` is not implemented for `ThreadRng`
    |         |
    |         required by a bound introduced by this call
    |::
    = help: the following other types implement trait `rand_core::RngCore`:
...
note: required by a bound in `Mnemonic::generate_in_with`
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:266:6
    |
260 |     pub fn generate_in_with<R>(
    |            ---------------- required by a bound in this associated function
...
266 |         R: rand_core::RngCore + rand_core::CryptoRng,
    |            ^^^^^^^^^^^^^^^^^^ required by this bound in `Mnemonic::generate_in_with`

```

```
error[E0277]: the trait bound `ThreadRng: rand_core::CryptoRng` is not satisfied
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:292:30
    |
292 |         Mnemonic::generate_in_with(&mut rand::thread_rng(), language, word_count)
    |         -------------------------- ^^^^^^^^^^^^^^^^^^^^^^^ the trait `rand_core::CryptoRng` is not implemented for `ThreadRng`
    |         |
    |         required by a bound introduced by this call
    |
    = help: the following other types implement trait `rand_core::CryptoRng`:
...
note: required by a bound in `Mnemonic::generate_in_with`
   --> /home/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bip39-2.0.0/src/lib.rs:266:27
    |
260 |     pub fn generate_in_with<R>(
    |            ---------------- required by a bound in this associated function
...
266 |         R: rand_core::RngCore + rand_core::CryptoRng,
    |                                 ^^^^^^^^^^^^^^^^^^^^ required by this bound in `Mnemonic::generate_in_with`

```

Co-authored-by: Tobin C. Harding <[email protected]>
@michalkucharczyk
Copy link
Copy Markdown
Contributor Author

Can you rebase on master to pass CI?

Done (force push)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants